-
Notifications
You must be signed in to change notification settings - Fork 37
Fix and refactor checks retry #7456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ry-checks-IFC-1023
WalkthroughUpdated VALIDATIONS_ENUM_MAP to use CheckType and added CoreGeneratorValidator = "GENERATOR". Added single-id GraphQL API helpers: getValidatorsFromApi, getCheckDetailsFromApi, and runCheckFromApi; removed the Handlebars runCheck template. Introduced domain wrappers and React Query hooks: getValidators, getCheckDetails, runCheck, useGetValidatorsQuery, useGetCheckDetails, and useRunCheckMutation, plus new query keys. Migrated UI components (Checks, ChecksSummary, Check) from Apollo GraphQL useQuery/mutation flows to the new domain hooks and mutation hooks. PieChart props renamed and children removed; minor UI adjustments in checks summary. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/src/entities/diff/checks/checks-summary.tsx (1)
86-95: Disable “Retry all” during mutation and reflect pending state.Prevents duplicate submissions and keeps UI consistent with per‑chart retry.
- <Button - onClick={() => handleRetry("all")} - disabled={!isAuthenticated} + <Button + onClick={() => handleRetry("all")} + disabled={!isAuthenticated || isPending} variant="ghost" className="gap-1 hover:bg-neutral-200" > Retry all - <Icon icon="mdi:reload" className={classNames(isLoading && "animate-spin")} /> + <Icon icon="mdi:reload" className={classNames((isPending || isLoading) && "animate-spin")} /> </Button>
🧹 Nitpick comments (7)
frontend/app/src/shared/components/display/pie-chart.tsx (1)
38-38: Remove unnecessary empty className.The empty string className adds no value and creates noise in the DOM.
Apply this diff to clean up:
- <RPieChart width={100} height={60} className=""> + <RPieChart width={100} height={60}>frontend/app/src/entities/diff/checks/checks.tsx (1)
34-34: Optional chaining is defensive but unnecessary.After the
isPendingcheck on line 25,validatorsshould always be defined. The optional chainingvalidators?.mapis safe but suggests uncertainty about the data shape.If
validatorsis always defined after loading, consider removing the optional chaining for clarity:- {validators?.map((item: any) => ( + {validators.map((item: any) => (frontend/app/src/entities/diff/api/get-validators-from-api.ts (3)
5-7: Prefer a list variable over an inline array literal.Using
[$id]is valid GraphQL, but passing a dedicated list variable is clearer and easier to reuse.Proposed:
- query GET_CORE_VALIDATORS($id: ID!) { - CoreValidator(proposed_change__ids: [$id]) { + query GET_CORE_VALIDATORS($ids: [ID!]!) { + CoreValidator(proposed_change__ids: $ids) {And:
- variables: { id: proposedChangeId }, + variables: { ids: [proposedChangeId] },
5-6: Align names: constant vs operation.
const GET_VALIDATORSwrapsquery GET_CORE_VALIDATORS. Consider renaming one to match the other for grep-ability.
43-54: Add typed response/variables (if codegen is available).Return a typed result (e.g., generated
GET_CORE_VALIDATORSQuery) to avoidanydownstream. If using GraphQL Code Generator, import the typed document and set the generic onquery<Resp, Vars>.frontend/app/src/entities/diff/checks/checks-summary.tsx (2)
103-103: Avoid passing a boolean to onClick.Pass
undefinedwhen disabled to satisfy expected handler type and avoid accidental booleans.- <PieChart data={data} onClick={() => canRetry(data) && handleRetry(kind)}> + <PieChart data={data} onClick={canRetry(data) ? () => handleRetry(kind) : undefined}>
105-111: Improve retry overlay accessibility.Make the hover‑only control keyboard‑accessible with semantics; optional but recommended.
- <div className="invisible absolute group-hover:visible"> + <div + className="invisible absolute group-hover:visible" + role="button" + tabIndex={0} + aria-label={`Retry ${schemaKindLabel[kind]?.replace("Validator", "").trim()} checks`} + onKeyDown={(e) => { + if ((e.key === "Enter" || e.key === " ") && canRetry(data)) handleRetry(kind); + }} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/app/src/config/constants.ts(1 hunks)frontend/app/src/entities/diff/api/get-validators-from-api.ts(2 hunks)frontend/app/src/entities/diff/api/run-check-from-api.ts(1 hunks)frontend/app/src/entities/diff/api/runCheck.ts(0 hunks)frontend/app/src/entities/diff/checks/checks-summary.tsx(4 hunks)frontend/app/src/entities/diff/checks/checks.tsx(1 hunks)frontend/app/src/entities/diff/domain/diff.query-keys.ts(1 hunks)frontend/app/src/entities/diff/domain/get-validators.query.ts(1 hunks)frontend/app/src/entities/diff/domain/get-validators.ts(1 hunks)frontend/app/src/entities/diff/domain/run-check.mutation.ts(1 hunks)frontend/app/src/entities/diff/domain/run-check.ts(1 hunks)frontend/app/src/shared/components/display/pie-chart.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/app/src/entities/diff/api/runCheck.ts
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/app/src/entities/diff/domain/get-validators.query.ts (3)
frontend/app/src/entities/diff/api/get-validators-from-api.ts (1)
GetValidatorsFromApiParams(43-45)frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
proposedChangeValidatorsKeys(17-23)frontend/app/src/entities/diff/domain/get-validators.ts (1)
getValidators(6-10)
frontend/app/src/entities/diff/domain/get-validators.ts (1)
frontend/app/src/entities/diff/api/get-validators-from-api.ts (2)
GetValidatorsFromApiParams(43-45)getValidatorsFromApi(47-54)
frontend/app/src/entities/diff/domain/run-check.mutation.ts (3)
frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
runCheckMutationKeys(13-15)frontend/app/src/entities/diff/api/run-check-from-api.ts (1)
UpdateCheckFromApiParams(18-21)frontend/app/src/entities/diff/domain/run-check.ts (1)
runCheck(5-7)
frontend/app/src/entities/diff/domain/run-check.ts (1)
frontend/app/src/entities/diff/api/run-check-from-api.ts (2)
UpdateCheckFromApiParams(18-21)runCheckFromApi(23-31)
frontend/app/src/entities/diff/checks/checks-summary.tsx (4)
frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
useRunCheckMutation(7-12)frontend/app/src/shared/api/rest/client.ts (1)
queryClient(11-17)frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
proposedChangeValidatorsKeys(17-23)frontend/app/src/shared/components/buttons/retry.tsx (1)
Retry(12-44)
frontend/app/src/entities/diff/checks/checks.tsx (4)
frontend/app/src/entities/diff/domain/get-validators.query.ts (1)
useGetValidatorsQuery(7-14)frontend/app/src/shared/components/errors/error-screen.tsx (1)
ErrorScreen(15-26)frontend/app/src/entities/diff/checks/checks-summary.tsx (1)
ChecksSummary(33-124)frontend/app/src/entities/diff/checks/validator.tsx (1)
Validator(66-133)
🔇 Additional comments (7)
frontend/app/src/config/constants.ts (1)
121-121: LGTM! Validator mapping added correctly.The addition of
CoreGeneratorValidator: "GENERATOR"to the enum map is consistent with existing validator mappings and aligns with theCoreGeneratorValidatortype already present inMENU_EXCLUDELIST(line 77).frontend/app/src/entities/diff/domain/get-validators.query.ts (1)
1-14: LGTM!The query hook follows React Query best practices with proper query key scoping and type-safe
queryOptionsusage.frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
1-12: LGTM!The mutation hook is well-structured and follows React Query conventions. Error handling is appropriately delegated to consumers.
frontend/app/src/entities/diff/domain/get-validators.ts (1)
1-10: LGTM!The function safely handles edge cases with optional chaining and provides a sensible default empty array.
frontend/app/src/entities/diff/domain/run-check.ts (1)
1-7: LGTM!The domain wrapper appropriately abstracts the API layer and provides a clean interface with a semantic type alias.
frontend/app/src/entities/diff/checks/checks.tsx (2)
10-19: Excellent refactor from imperative to declarative pattern.Removing
forwardRefand usinguseGetValidatorsQuerysimplifies the component and aligns with React Query best practices.
25-27: Good loading state handling.The early return with
LoadingIndicatorimproves UX and prevents rendering issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/app/src/entities/diff/domain/diff.query-keys.ts (2)
22-23: Addas constassertion for type safety.The
proposedChangeValidatorsKeys.allarray should useas constto match the pattern of other query keys in this file (lines 4, 10, 14, 19) and ensure proper type inference.Apply this diff:
export const proposedChangeValidatorsKeys = { - all: ["proposed-change-validators"], + all: ["proposed-change-validators"] as const, allWithinProposedChange: (proposedChangeId: string) => [
24-27: Addas constassertion for type safety.The
allWithinProposedChangefunction should return a readonly tuple by addingas constto match the pattern on line 6.Apply this diff:
allWithinProposedChange: (proposedChangeId: string) => [ ...proposedChangeValidatorsKeys.all, proposedChangeId, - ], + ] as const, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
changelog/+checks.fixed.md(1 hunks)frontend/app/src/entities/diff/api/get-check-details-from-api.ts(2 hunks)frontend/app/src/entities/diff/checks/check.tsx(3 hunks)frontend/app/src/entities/diff/domain/diff.query-keys.ts(1 hunks)frontend/app/src/entities/diff/domain/get-check-details.query.ts(1 hunks)frontend/app/src/entities/diff/domain/get-check-details.ts(1 hunks)frontend/app/src/entities/diff/domain/get-validators.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- changelog/+checks.fixed.md
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/src/entities/diff/domain/get-validators.ts
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/app/src/entities/diff/domain/get-check-details.query.ts (3)
frontend/app/src/entities/diff/api/get-check-details-from-api.ts (1)
GetCheckDetailsFromApiParams(69-71)frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
getCheckQueryKeys(13-16)frontend/app/src/entities/diff/domain/get-check-details.ts (1)
getCheckDetails(6-12)
frontend/app/src/entities/diff/domain/get-check-details.ts (1)
frontend/app/src/entities/diff/api/get-check-details-from-api.ts (2)
GetCheckDetailsFromApiParams(69-71)getCheckDetailsFromApi(73-80)
frontend/app/src/entities/diff/checks/check.tsx (3)
frontend/app/src/entities/diff/domain/get-check-details.query.ts (1)
useGetCheckDetailsQuery(7-14)frontend/app/src/shared/components/errors/error-screen.tsx (1)
ErrorScreen(15-26)frontend/app/src/shared/components/display/date-display.tsx (1)
DateDisplay(17-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverall-report
- GitHub Check: documentation
🔇 Additional comments (5)
frontend/app/src/entities/diff/domain/get-check-details.query.ts (1)
7-14: LGTM!The hook is well-structured and follows React Query best practices. The use of
queryOptionsensures proper type inference.frontend/app/src/entities/diff/api/get-check-details-from-api.ts (2)
5-67: LGTM!The GraphQL query is comprehensive and covers all necessary fields including type-specific fragments for CoreDataCheck, CoreSchemaCheck, CoreFileCheck, and CoreArtifactCheck.
69-80: LGTM!The interface and function implementation are clean and follow established patterns in the codebase.
frontend/app/src/entities/diff/checks/check.tsx (2)
77-89: LGTM!The migration from Apollo's
useQueryto the domain-specificuseGetCheckDetailsQueryhook is well-executed. Error and loading states are properly handled with dedicated components, improving user experience.
137-142: LGTM!The data access patterns correctly use optional chaining and provide appropriate fallbacks (
display_labelwhennameis unavailable, conditional rendering forcreated_at).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
frontend/app/src/shared/components/display/pie-chart.tsx (1)
32-32: Restore click affordance and add keyboard/a11y semantics when clickable.Bring back the pointer cursor and make the div operable via Enter/Space only when
onClickexists.- <div className={"relative"} onClick={handleClick}> + <div + className={`relative ${onClick ? "cursor-pointer" : ""}`} + onClick={handleClick} + role={onClick ? "button" : undefined} + tabIndex={onClick ? 0 : undefined} + onKeyDown={(e) => { + if (!onClick) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onClick(); + } + }} + >frontend/app/src/entities/diff/checks/checks-summary.tsx (1)
51-66: Harden handleRetry: remove non-null assertions, add guards, reentrancy block, and onError toast.Unsafe
!onproposedChangeIdandVALIDATIONS_ENUM_MAP[validator], no auth/ID checks, noonError, and re-entrancy not blocked. Alsoasyncis unused. Apply:- const handleRetry = async (validator: string) => { - mutate( - { - proposedChangeId: proposedChangeId!, - checkType: VALIDATIONS_ENUM_MAP[validator]!, - }, - { - onSuccess: () => { - queryClient.invalidateQueries({ - queryKey: proposedChangeValidatorsKeys.allWithinProposedChange(proposedChangeId!), - }); - toast(<Alert type={ALERT_TYPES.SUCCESS} message="Checks are running" />); - }, - } - ); - }; + const handleRetry = (validator: keyof typeof VALIDATIONS_ENUM_MAP) => { + if (isPending) return; + if (!isAuthenticated) { + toast(<Alert type={ALERT_TYPES.ERROR} message="You must be signed in to retry checks" />); + return; + } + const pcId = proposedChangeId; + if (!pcId) { + toast(<Alert type={ALERT_TYPES.ERROR} message="Missing proposed change id" />); + return; + } + const checkType = VALIDATIONS_ENUM_MAP[validator]; + if (!checkType) { + toast(<Alert type={ALERT_TYPES.ERROR} message={`Unknown validator: ${String(validator)}`} />); + return; + } + mutate( + { proposedChangeId: pcId, checkType }, + { + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: proposedChangeValidatorsKeys.allWithinProposedChange(pcId), + }); + toast(<Alert type={ALERT_TYPES.SUCCESS} message="Checks are running" />); + }, + onError: () => { + toast(<Alert type={ALERT_TYPES.ERROR} message="Failed to start checks" />); + }, + } + ); + };
🧹 Nitpick comments (4)
frontend/app/src/shared/components/display/pie-chart.tsx (3)
3-6: Tighten prop types (avoidany[]andFunction).Stronger typing improves safety and editor help.
-type PieChartProps = { - data: any[]; - onClick?: Function; -}; +type SliceDatum = { name: string; value: number; className?: string }; +type PieChartProps = { + data: SliceDatum[]; + onClick?: () => void; +};
22-24: Minor: inline destructuring is fine; consider typing handlers for clarity.
No behavior change, but adding explicit types helps future edits.-export const PieChart = (props: PieChartProps) => { - const { data, onClick } = props; +export const PieChart = ({ data, onClick }: PieChartProps) => { + const handleClick: () => void = () => { + if (!onClick) return; + onClick(); + };
1-1: Alias Recharts Tooltip to avoid confusion with app UI Tooltip.Project also exports a
Tooltipcomponent (frontend/app/src/shared/components/ui/tooltip.tsx). Alias the Recharts one to make intent obvious.-import { Cell, Pie, PieChart as RPieChart, Tooltip } from "recharts"; +import { Cell, Pie, PieChart as RPieChart, Tooltip as RechartsTooltip } from "recharts"; ... - <Tooltip content={renderCustomizedTooltip} /> + <RechartsTooltip content={renderCustomizedTooltip} />If both tooltips appear in this module in future, this avoids naming collisions. Based on relevant UI tooltip file.
Also applies to: 47-47
frontend/app/src/entities/diff/checks/checks-summary.tsx (1)
104-116: A11y: reveal controls on keyboard focus, not only hover.Add focus styles so keyboard users can access Retry; today it's hover-only.
- className={classNames( - "text-xs", - canRetry(data) && "absolute text-xs group-hover:invisible" - )} + className={classNames( + "text-xs", + canRetry(data) && "absolute text-xs group-hover:invisible group-focus-within:invisible" + )} ... - <div className="invisible absolute group-hover:visible"> + <div className="invisible absolute group-hover:visible group-focus-within:visible">Consider updating Retry to render a native with tabIndex, role, and keyboard handlers. I can draft that if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/src/entities/diff/checks/checks-summary.tsx(5 hunks)frontend/app/src/shared/components/display/pie-chart.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/app/src/shared/components/display/pie-chart.tsx (1)
frontend/app/src/shared/components/ui/tooltip.tsx (1)
Tooltip(11-40)
frontend/app/src/entities/diff/checks/checks-summary.tsx (10)
frontend/app/src/entities/schema/stores/schema.atom.ts (1)
genericSchemasAtom(12-12)frontend/app/src/entities/authentication/ui/useAuth.tsx (1)
useAuth(173-175)frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
useRunCheckMutation(7-12)frontend/app/src/config/constants.ts (1)
VALIDATIONS_ENUM_MAP(118-126)frontend/app/src/shared/api/rest/client.ts (1)
queryClient(11-17)frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
proposedChangeValidatorsKeys(22-26)frontend/app/src/shared/components/ui/alert.tsx (1)
Alert(31-142)frontend/app/src/shared/components/buttons/retry.tsx (1)
Retry(12-44)frontend/app/src/shared/components/display/pie-chart.tsx (1)
PieChart(22-51)frontend/app/src/shared/utils/common.ts (1)
classNames(5-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E-testing-version-upgrade / From 1.3.0
- GitHub Check: E2E-testing-playwright
- GitHub Check: E2E-testing-invoke-demo-start
- GitHub Check: documentation
🔇 Additional comments (3)
frontend/app/src/entities/diff/checks/checks-summary.tsx (3)
11-11: Wiring looks good.Imports of queryClient, mutation hook, and query keys + use of isPending are appropriate.
Also applies to: 20-20, 25-25, 39-39
102-102: Decoupling chart click from retry is good.Cleaner separation; Retry is now explicit.
11-11: Single shared QueryClient instance confirmed.Verification shows both
QueryClientProviderinstances (inapp.tsxandrender.tsx) import and use the samequeryClientexported from@/shared/api/rest/client. The import inchecks-summary.tsxreferences the identical instance—invalidation will fire correctly.
changelog/+checks.fixed.md
Outdated
| @@ -0,0 +1 @@ | |||
| Fix check retry request and loading state No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changelog is not by user oriented. We write it for them
…frahub into ple-fix-retry-checks-IFC-1023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
frontend/app/src/entities/diff/checks/checks-summary.tsx (3)
26-29: Tighten prop types (avoid any[]).
Use the minimal readonly shape you actually need (__typename).-type ChecksSummaryProps = { - validators: any[]; - isLoading: boolean; -}; +type ChecksSummaryProps = { + validators: ReadonlyArray<{ __typename: string }>; + isLoading: boolean; +};
50-64: Harden mutation: remove non‑null assertions, add guards and onError.
Prevent runtime throws and surface failures to users.- const handleRetry = async (validator: string) => { - mutate( - { - proposedChangeId: proposedChangeId!, - checkType: VALIDATIONS_ENUM_MAP[validator]!, - }, - { - onSuccess: () => { - queryClient.invalidateQueries({ - queryKey: proposedChangeValidatorsKeys.allWithinProposedChange(proposedChangeId!), - }); - toast(<Alert type={ALERT_TYPES.SUCCESS} message="Checks are running" />); - }, - } - ); - }; + const handleRetry = async (validator: string) => { + if (isPending) return; + const pcId = proposedChangeId; + if (!pcId) { + toast(<Alert type={ALERT_TYPES.ERROR} message="Missing proposed change id" />); + return; + } + const checkType = VALIDATIONS_ENUM_MAP[validator as keyof typeof VALIDATIONS_ENUM_MAP]; + if (!checkType) { + toast(<Alert type={ALERT_TYPES.ERROR} message={`Unknown validator: ${validator}`} />); + return; + } + mutate( + { proposedChangeId: pcId, checkType }, + { + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: proposedChangeValidatorsKeys.allWithinProposedChange(pcId), + }); + toast(<Alert type={ALERT_TYPES.SUCCESS} message="Checks are running" />); + }, + onError: () => { + toast(<Alert type={ALERT_TYPES.ERROR} message="Failed to start checks" />); + }, + } + ); + };
113-121: Per‑validator Retry: gate by auth; compute in‑progress from stats.
data.inProgressdoesn’t exist; also block unauthenticated clicks.{canRetry(data) && ( <div className="invisible absolute group-hover:visible"> <Retry - isLoading={isPending || isLoading || !!data.inProgress} - isDisabled={!canRetry(data)} - onClick={() => canRetry(data) && handleRetry(kind)} + isLoading={ + isPending || + isLoading || + data.some((s: any) => s.name === CHECKS_LABEL.IN_PROGRESS && !!(s as any).value) + } + isDisabled={!isAuthenticated || !canRetry(data)} + onClick={() => isAuthenticated && canRetry(data) && handleRetry(kind)} /> </div> )}
🧹 Nitpick comments (2)
frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
5-9: Type the mutation and allow options pass-through for better DX.Annotate generics so mutate() is correctly typed, and optionally accept options to extend handlers.
-import { useMutation } from "@tanstack/react-query"; +import { useMutation, type UseMutationOptions } from "@tanstack/react-query"; +import type { CheckType } from "@/shared/api/graphql/generated/graphql"; import { runCheck } from "@/entities/diff/domain/run-check"; -export function useRunCheckMutation() { - return useMutation({ - mutationFn: runCheck, - }); -} +type Vars = { proposedChangeId: string; checkType: CheckType }; +export function useRunCheckMutation( + options?: UseMutationOptions<void, unknown, Vars, unknown> +) { + return useMutation<void, unknown, Vars>({ + mutationFn: runCheck, + ...options, + }); +}frontend/app/src/entities/diff/checks/checks.tsx (1)
10-19: Optional: guard missing route param instead of non‑null assertion.Avoid
proposedChangeId!; render an error if absent.export const Checks = () => { const { proposedChangeId } = useParams(); - const { + if (!proposedChangeId) { + return <ErrorScreen message="Missing proposed change id." />; + } + + const { isPending, error, data: validators, } = useGetValidatorsQuery({ - proposedChangeId: proposedChangeId!, + proposedChangeId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
changelog/+checks.fixed.md(1 hunks)frontend/app/src/config/constants.ts(2 hunks)frontend/app/src/entities/diff/api/get-check-details-from-api.ts(2 hunks)frontend/app/src/entities/diff/api/get-validators-from-api.ts(1 hunks)frontend/app/src/entities/diff/api/run-check-from-api.ts(1 hunks)frontend/app/src/entities/diff/checks/check.tsx(3 hunks)frontend/app/src/entities/diff/checks/checks-summary.tsx(5 hunks)frontend/app/src/entities/diff/checks/checks.tsx(1 hunks)frontend/app/src/entities/diff/checks/validator-details.tsx(0 hunks)frontend/app/src/entities/diff/domain/diff.query-keys.ts(1 hunks)frontend/app/src/entities/diff/domain/get-check-details.query.ts(1 hunks)frontend/app/src/entities/diff/domain/get-check-details.ts(1 hunks)frontend/app/src/entities/diff/domain/get-validators.query.ts(1 hunks)frontend/app/src/entities/diff/domain/get-validators.ts(1 hunks)frontend/app/src/entities/diff/domain/run-check.mutation.ts(1 hunks)frontend/app/src/entities/diff/domain/run-check.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/app/src/entities/diff/checks/validator-details.tsx
✅ Files skipped from review due to trivial changes (1)
- changelog/+checks.fixed.md
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/app/src/entities/diff/domain/get-validators.query.ts
- frontend/app/src/entities/diff/domain/diff.query-keys.ts
- frontend/app/src/entities/diff/api/get-check-details-from-api.ts
- frontend/app/src/config/constants.ts
- frontend/app/src/entities/diff/api/get-validators-from-api.ts
- frontend/app/src/entities/diff/domain/get-check-details.ts
- frontend/app/src/entities/diff/domain/get-check-details.query.ts
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
frontend/app/src/entities/diff/domain/run-check.ts (1)
runCheck(11-13)
frontend/app/src/entities/diff/domain/get-validators.ts (1)
frontend/app/src/entities/diff/api/get-validators-from-api.ts (1)
getValidatorsFromApi(49-54)
frontend/app/src/entities/diff/domain/run-check.ts (1)
frontend/app/src/entities/diff/api/run-check-from-api.ts (1)
runCheckFromApi(24-32)
frontend/app/src/entities/diff/checks/checks-summary.tsx (9)
frontend/app/src/entities/schema/stores/schema.atom.ts (1)
genericSchemasAtom(12-12)frontend/app/src/entities/authentication/ui/useAuth.tsx (1)
useAuth(173-175)frontend/app/src/entities/diff/domain/run-check.mutation.ts (1)
useRunCheckMutation(5-9)frontend/app/src/config/constants.ts (1)
VALIDATIONS_ENUM_MAP(120-128)frontend/app/src/shared/api/rest/client.ts (1)
queryClient(11-17)frontend/app/src/entities/diff/domain/diff.query-keys.ts (1)
proposedChangeValidatorsKeys(18-22)frontend/app/src/shared/components/buttons/retry.tsx (1)
Retry(12-44)frontend/app/src/shared/components/display/pie-chart.tsx (1)
PieChart(22-51)frontend/app/src/shared/utils/common.ts (1)
classNames(5-7)
frontend/app/src/entities/diff/checks/checks.tsx (4)
frontend/app/src/entities/diff/domain/get-validators.query.ts (1)
useGetValidatorsQuery(6-13)frontend/app/src/shared/components/errors/error-screen.tsx (1)
ErrorScreen(15-26)frontend/app/src/entities/diff/checks/checks-summary.tsx (1)
ChecksSummary(31-130)frontend/app/src/entities/diff/checks/validator.tsx (1)
Validator(66-133)
frontend/app/src/entities/diff/checks/check.tsx (3)
frontend/app/src/entities/diff/domain/get-check-details.query.ts (1)
useGetCheckDetails(6-13)frontend/app/src/shared/components/errors/error-screen.tsx (1)
ErrorScreen(15-26)frontend/app/src/shared/components/display/date-display.tsx (1)
DateDisplay(17-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: E2E-testing-version-upgrade / From 1.3.0
- GitHub Check: E2E-testing-invoke-demo-start
- GitHub Check: frontend-tests
- GitHub Check: E2E-testing-playwright
- GitHub Check: documentation
- GitHub Check: validate-generated-documentation
- GitHub Check: backend-validate-generated
🔇 Additional comments (2)
frontend/app/src/entities/diff/api/run-check-from-api.ts (1)
9-20: LGTM — mutation wiring and types look consistent.
Using generated types and passing variables explicitly keeps this safe and maintainable.Also applies to: 22-32
frontend/app/src/entities/diff/domain/run-check.ts (1)
5-13: LGTM — type-safe domain wrapper.
Clear boundary and correct use of generated CheckType.
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes
Chores